Add Multicall3 deployment and pagination support for Ethscriptions#134
Conversation
- Implemented the deployment of the Multicall3 contract in L2Genesis. - Introduced pagination structures and methods in the Ethscriptions contract for efficient batch queries. - Added tests for Multicall3 functionality, including deployment verification and mixed call scenarios. - Enhanced error handling for pagination limits and added relevant constants.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds Multicall3 support and pagination helpers to the Ethscriptions protocol. The main purpose is to enable efficient batch querying of ethscriptions data, improving the developer experience for off-chain clients and indexers.
- Deploys Multicall3 at the canonical address for batch contract calls
- Adds pagination functions to query ethscriptions globally or by owner
- Includes comprehensive test coverage for Multicall3 integration
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| contracts/test/Multicall3.t.sol | New test suite validating Multicall3 deployment and batch querying of ethscriptions |
| contracts/src/libraries/Predeploys.sol | Adds Multicall3 predeploy address constant and bytecode for deployment |
| contracts/src/Ethscriptions.sol | Implements pagination helpers with configurable limits and includeContent flags |
| contracts/script/L2Genesis.s.sol | Integrates Multicall3 deployment into genesis state setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -18,6 +18,8 @@ library Predeploys { | |||
| /// @notice ProxyAdmin predeploy (manages all proxy upgrades) | |||
| address constant PROXY_ADMIN = 0x4200000000000000000000000000000000000018; | |||
|
|
|||
There was a problem hiding this comment.
Missing NatSpec documentation for the MultiCall3 constant. Other constants in this file have /// @notice comments explaining their purpose. Add documentation such as: /// @notice Multicall3 predeploy (canonical multicall contract for batched calls)
| /// @notice Multicall3 predeploy (canonical multicall contract for batched calls) |
| uint160(uint256(uint160(_addr)) & 0xffff | uint256(uint160(0xc0D3C0d3C0d3C0D3c0d3C0d3c0D3C0d3c0d30000))) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing NatSpec documentation for the MultiCall3Code constant. Add a comment explaining what this bytecode represents, such as: /// @notice Compiled bytecode for the Multicall3 contract deployment
| /// @notice Compiled bytecode for the Multicall3 contract deployment |
| /// @dev Maximum page sizes for pagination helpers | ||
| uint256 private constant MAX_PAGE_WITH_CONTENT = 50; |
There was a problem hiding this comment.
Missing NatSpec documentation explaining why these specific page size limits were chosen. Consider adding comments that explain the rationale (e.g., gas limits, response size considerations) for these magic numbers.
| /// @dev Maximum page sizes for pagination helpers | |
| uint256 private constant MAX_PAGE_WITH_CONTENT = 50; | |
| /// @dev Maximum page sizes for pagination helpers | |
| /** | |
| * @dev Maximum page size for pagination when content is included. | |
| * Chosen as 50 to balance between gas cost and response size, ensuring | |
| * that returning content for each item does not exceed block gas limits | |
| * or cause out-of-gas errors in typical use cases. | |
| */ | |
| uint256 private constant MAX_PAGE_WITH_CONTENT = 50; | |
| /** | |
| * @dev Maximum page size for pagination when content is NOT included. | |
| * Set to 1000 since omitting content allows for much larger page sizes | |
| * without risking excessive gas usage or response size. This value was | |
| * chosen to support efficient bulk queries while remaining safe for on-chain execution. | |
| */ |
|
bugbot run |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| function testGas_CheckLimitsWork() public view { |
There was a problem hiding this comment.
The function testGas_CheckLimitsWork is marked as view but uses require statements for assertions. In Forge tests, require should be replaced with assertLe from the Test contract for proper test failure reporting and consistency with other test files that use assertEq, assertTrue, etc.
| // Test without content - should clamp at 1000 | ||
| result = ethscriptions.getEthscriptions(0, 5000, false); | ||
| console.log("Requested 5000 without content, got:", result.items.length); | ||
| require(result.items.length <= 1000, "Should clamp to 1000"); |
There was a problem hiding this comment.
Replace require with assertLe(result.items.length, 1000, \"Should clamp to 1000\") for consistent test assertion patterns. The Forge testing framework provides dedicated assertion functions that give better error messages and integrate properly with test runners.
| require(result.items.length <= 1000, "Should clamp to 1000"); | |
| assertLe(result.items.length, 1000, "Should clamp to 1000"); |
| // Test with content - should clamp at 50 | ||
| result = ethscriptions.getEthscriptions(0, 500, true); | ||
| console.log("Requested 500 with content, got:", result.items.length); | ||
| require(result.items.length <= 50, "Should clamp to 50"); |
There was a problem hiding this comment.
Replace require with assertLe(result.items.length, 50, \"Should clamp to 50\") for consistent test assertion patterns. The Forge testing framework provides dedicated assertion functions that give better error messages and integrate properly with test runners.
| require(result.items.length <= 50, "Should clamp to 50"); | |
| assertLe(result.items.length, 50, "Should clamp to 50"); |
Note
Adds Multicall3 as a predeploy in L2 genesis and introduces paginated Ethscriptions query endpoints with size limits, with comprehensive tests.
Multicall3inL2Genesis.s.solvia newdeployMulticall3()and call it during setup.Predeploys.MultiCall3address and embeddedMultiCall3Codebytecode.PaginatedEthscriptionsResponsestruct,getEthscriptions(start, limit[, includeContent]),getOwnerEthscriptions(owner, start, limit[, includeContent]).MAX_PAGE_WITH_CONTENT=50,MAX_PAGE_WITHOUT_CONTENT=1000) andInvalidPaginationLimiterror._createPaginatedEthscriptionsResponsesupporting optional content inclusion.Multicall3.t.sol: verify deployment and batch calls (mixed success/failure).PaginationGas.t.sol,PaginationGas1000.t.sol,PaginationGas1000Simple.t.solfor page sizes, clamping, and edge cases.Written by Cursor Bugbot for commit 7d61c9f. This will update automatically on new commits. Configure here.